Add item groups (aka creative tabs) · Pull Request #1866 · SpongePowered/SpongeAPI

ghost · 2018-07-17T01:37:39Z

SpongeAPI | SpongeCommon | SpongeForge

Added ItemGroup interface, cataloged by ItemGroups
Added Optional<ItemGroup> getItemGroup() to BlockType and ItemType

Granted, creative tabs are not super important on the server, but it could be useful, for example, for detecting mod-added tabs.

_{have mercy on me, this is my first sponge pull request}

kashike · 2018-07-17T01:52:37Z

+     *
+     * @return The item group or {@link Optional#empty()} otherwise
+     */
+    Optional<ItemGroup> getItemGroup();


Remove this method - it be can retrieved from getItem() instead.

mattmess1221 · 2018-07-17T02:56:33Z

+     *
+     * @return The item group or {@link Optional#empty()} otherwise
+     */
+    Optional<ItemGroup> getItemGroup();


An item can be in multiple groups.

Quite possibly needs to be a Collection<ItemGroup>, since Collections.empty() is easy to call.

liach · 2018-07-17T03:04:05Z

+ * Represents an item group, or creative tab.
+ */
+@CatalogedBy(ItemGroups.class)
+public interface ItemGroup extends CatalogType, Translatable {}


Braces on different lines and leave a blank line between. Also, can the icon of the item group be exposed?

You should add a method to see if an item is in this item group possibly.

Unfortunately, item group icons are client only.

At the very least, make the class ending brace on a new line and leave an empty line in the middle.

liach · 2018-07-17T03:04:44Z

+
+    public static final ItemGroup TRANSPORTATION = DummyObjectProvider.createFor(ItemGroup.class, "TRANSPORTATION");
+
+    // SORTFIELDS:OFF


You missed the SEARCH item group.

Yes, SEARCH, along with HOTBAR and INVENTORY, were skipped because they are not so much item groups as they are creative tabs.

Are you attempting to expose the tab itself, for say, GUI click events.

Or are you attempting to expose the property of items to belong to a creative tab?

Yeah, just exposing the property of items to belong to an item group.

gabizou

Quite possibly could be named ItemCollection instead of ItemGroup. I'm impartial to either name, but it is something we can expose at least in writing but without any functionality.

gabizou · 2018-07-17T04:56:25Z

+ * Represents an item group, or creative tab.
+ */
+@CatalogedBy(ItemGroups.class)
+public interface ItemGroup extends CatalogType, Translatable {}


At the very least, make the class ending brace on a new line and leave an empty line in the middle.

gabizou · 2018-07-17T04:58:06Z

+import org.spongepowered.api.util.annotation.CatalogedBy;
+
+/**
+ * Represents an item group, or creative tab.


How would this compare to being used? They're just signified by where they show on clients? There's otherwise no other usable functionality to gain from this, right?

Should document by answering these questions in some sense. That will fill out the javadoc for this class.

gabizou · 2018-07-17T04:59:49Z

+import org.spongepowered.api.util.generator.dummy.DummyObjectProvider;
+
+/**
+ * An enumeration of all possible {@link ItemGroup}s in vanilla minecraft.


Maybe phrase it as An enumeration of the known vanilla {@link ItemGroup}s in Minecraft. Mods may change and/or introduce new groups at runtime.

gabizou · 2018-07-17T05:00:32Z

+     *
+     * @return The item group or {@link Optional#empty()} otherwise
+     */
+    Optional<ItemGroup> getItemGroup();


Quite possibly needs to be a Collection<ItemGroup>, since Collections.empty() is easy to call.

ghost · 2018-07-19T04:11:41Z

~wip

ghost · 2018-07-20T05:42:03Z

~qa

ryantheleach · 2018-07-20T07:11:45Z

@gbui Can you offer some usecases?

At the moment, without any sample usecases, or user stories, I'm not sure whether this API is sufficient or not.

I like that you have the possibility for an item to be in multiple groups.

@gabizou I'm not sold on ItemCollection as a name, it adds confusion with Java's Collections to make it seem like it's related to the Collections API / interfaces / classes.

XakepSDK · 2018-07-20T07:56:03Z

Why just not name it ItemGroup -> CreativeTab? ItemGroup may be confusing with 1.13 item tags.

ghost · 2018-07-20T14:45:51Z

@ryantheleach A sample usecase could be creating an in-game shopping catalog, organized by item group. This would make it easier for players to navigate the interface (which in this case would be an inventory "GUI").

@XakepSDK I went with ItemGroup since I'm just exposing the property of items to belong to an item group. CreativeTab is specific to the client side, while ItemGroup is a more general term that makes more sense on the server side.

kashike · 2018-07-20T20:00:52Z

@XakepSDK It's worth noting that we've renamed CreativeTabs to ItemGroup in MCP.

ryantheleach · 2018-07-23T04:29:35Z

@gbui That makes sense to me. Thanks.

ghost mentioned this pull request Jul 17, 2018

Add item groups (aka creative tabs) SpongePowered/Sponge#2000

Closed

kashike suggested changes Jul 17, 2018

View reviewed changes

mattmess1221 reviewed Jul 17, 2018

View reviewed changes

liach suggested changes Jul 17, 2018

View reviewed changes

gabizou requested changes Jul 17, 2018

View reviewed changes

ghost changed the title ~~Add item groups (aka creative tabs)~~ [WIP] Add item groups (aka creative tabs) Jul 17, 2018

limbo-app added the status: wip label Jul 19, 2018

ghost changed the title ~~[WIP] Add item groups (aka creative tabs)~~ Add item groups (aka creative tabs) Jul 19, 2018

gbui added 4 commits July 19, 2018 08:14

Add item groups (aka creative tabs)

1edebb7

Remove getItemGroup() in BlockType

e3ceb87

Add blank line between braces in ItemGroup

4d9c5b2

Replace getItemGroup() with getItemGroups()

02771f8

ghost mentioned this pull request Jul 19, 2018

Add item groups (aka creative tabs) SpongePowered/SpongeForge#2327

Closed

Make requested changes to docs

47a7995

limbo-app added status: needs review status: needs testing and removed status: wip labels Jul 20, 2018

gabizou added branch: bleeding api: 8 (u) version: 1.16 (unsupported since Oct 17th 2023) labels Aug 30, 2018

Zidane self-assigned this Feb 4, 2019

Zidane removed the branch: bleeding label Feb 4, 2019

ghost closed this Jul 30, 2019

ghost deleted the feature/itemgroups branch July 30, 2019 05:05

This pull request was closed.


		public static final ItemGroup TRANSPORTATION = DummyObjectProvider.createFor(ItemGroup.class, "TRANSPORTATION");

		// SORTFIELDS:OFF

Uh oh!

Conversation

ghost commented Jul 17, 2018 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gabizou left a comment • edited by ryantheleach Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ghost commented Jul 19, 2018

Uh oh!

ghost commented Jul 20, 2018

Uh oh!

ryantheleach commented Jul 20, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

XakepSDK commented Jul 20, 2018

Uh oh!

ghost commented Jul 20, 2018

Uh oh!

kashike commented Jul 20, 2018

Uh oh!

ryantheleach commented Jul 23, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

ghost commented Jul 17, 2018 •

edited by ghost

Loading

gabizou left a comment •

edited by ryantheleach

Loading

ryantheleach commented Jul 20, 2018 •

edited

Loading